add recoverymode and CRC checking to FIT reader. (#549)
authortsteven4 <13596209+tsteven4@users.noreply.github.com>
Fri, 1 May 2020 12:32:18 +0000 (06:32 -0600)
committerGitHub <noreply@github.com>
Fri, 1 May 2020 12:32:18 +0000 (06:32 -0600)
* add recoverymode and CRC checking to FIT reader.

If present, the header CRC is checked.
The file CRC and length is checked.

A recoverymode option is added.
In the default mode we will fatal with:
a bad CRC,
a bad endian field,
an attempt to read when the data section doesn't have sufficient data,
an unexepected EOF.
In recovery mode when we encounter one of these errors we will abort
read processing and continue.  This allows a more immediate cleaner
exit from the reader while still allowing any writer to use data that
was recovered previous to the read abort.

* add further explanation of recoverymode for document.

* make sure garmin fit messages are defined before being used.

garmin_fit.cc
garmin_fit.h
reference/format3.txt
reference/help.txt
testo.d/garmin_fit.test
xmldoc/formats/options/garmin_fit-recoverymode.xml [new file with mode: 0644]

index 32ccb3f79a1ab06438ced0f6f4745d4ad61fb656..d880056dac7bcb4ba5636899fb17ab33571ee1ea 100644 (file)
 
  */
 
-#include <cstdint>            // for uint8_t, uint16_t, uint32_t, int32_t, int8_t, uint64_t
-#include <cstdio>             // for EOF, SEEK_SET, snprintf
-#include <deque>              // for deque, _Deque_iterator, operator!=
-#include <memory>             // for allocator_traits<>::value_type
-#include <utility>            // for pair
-#include <vector>             // for vector
-
-#include <QtCore/QByteArray>  // for QByteArray
-#include <QtCore/QDateTime>   // for QDateTime
-#include <QtCore/QString>     // for QString
-#include <QtCore/Qt>          // for CaseInsensitive
+#include <cstdint>             // for uint8_t, uint16_t, uint32_t, int32_t, int8_t, uint64_t
+#include <cstdio>              // for EOF, SEEK_SET, snprintf
+#include <deque>               // for deque, _Deque_iterator, operator!=
+#include <memory>              // for allocator_traits<>::value_type
+#include <string>              // for operator+, to_string, char_traits
+#include <utility>             // for pair
+#include <vector>              // for vector
+
+#include <QtCore/QByteArray>   // for QByteArray
+#include <QtCore/QDateTime>    // for QDateTime
+#include <QtCore/QFileInfo>    // for QFileInfo
+#include <QtCore/QLatin1Char>  // for QLatin1Char
+#include <QtCore/QString>      // for QString
+#include <QtCore/Qt>           // for CaseInsensitive
+#include <QtCore/QtGlobal>     // for qint64
 
 #include "defs.h"
 #include "garmin_fit.h"
-#include "gbfile.h"           // for gbfputc, gbfputuint16, gbfputuint32, gbfgetc, gbfread, gbfseek, gbfclose, gbfgetuint16, gbfopen_le, gbfputint32, gbfflush, gbfgetuint32, gbfputs, gbftell, gbfwrite, gbfile, gbsize_t
-#include "jeeps/gpsmath.h"    // for GPS_Math_Semi_To_Deg, GPS_Math_Gtime_To_Utime, GPS_Math_Deg_To_Semi, GPS_Math_Utime_To_Gtime
+#include "gbfile.h"            // for gbfputc, gbfputuint16, gbfputuint32, gbfgetc, gbfread, gbfseek, gbfclose, gbfgetuint16, gbfopen_le, gbfputint32, gbfflush, gbfgetuint32, gbfputs, gbftell, gbfwrite, gbfile, gbsize_t
+#include "jeeps/gpsmath.h"     // for GPS_Math_Semi_To_Deg, GPS_Math_Gtime_To_Utime, GPS_Math_Deg_To_Semi, GPS_Math_Utime_To_Gtime
+#include "src/core/logging.h"  // for Warning, Fatal
 
 
 #define MYNAME "fit"
@@ -67,10 +72,7 @@ GarminFitFormat::rd_init(const QString& fname)
 void
 GarminFitFormat::rd_deinit()
 {
-  for (auto& local_id : fit_data.message_def) {
-    // QList::clear() will not deallocate
-    local_id.fields = QList<fit_field_t>();
-  }
+  fit_data = fit_data_t();
 
   gbfclose(fin);
 }
@@ -127,32 +129,57 @@ GarminFitFormat::fit_parse_header()
     debug_print(1,"%s: fit_data.len=%d\n", MYNAME, fit_data.len);
   }
 
-  if (len > 12) {
-    // Unused according to Ingo Arndt
-    gbfgetuint16(fin);
+  // Header CRC may be omitted entirely
+  if (len >= kReadHeaderCrcLen) {
+    uint16_t hdr_crc = gbfgetuint16(fin);
+    // Header CRC may be set to 0, or contain the CRC over previous bytes.
+    if (hdr_crc != 0) {
+      // Check the header CRC
+      uint16_t crc = 0;
+      gbfseek(fin, 0, SEEK_SET);
+      for (unsigned int i = 0; i < kReadHeaderCrcLen; ++i) {
+        int data = gbfgetc(fin);
+        if (data == EOF) {
+          fatal(MYNAME ": File %s truncated\n", fin->name);
+        }
+        crc = fit_crc16(data, crc);
+      }
+      if (crc != 0) {
+        Warning().nospace() << MYNAME ": Header CRC mismatch in file " <<  fin->name << ".";
+        if (!opt_recoverymode) {
+          Fatal().nospace() << MYNAME ": File " << fin->name << " is corrupt.  Use recoverymode option at your risk.";
+        }
+      } else if (global_opts.debug_level >= 1) {
+        debug_print(1, MYNAME ": Header CRC verified.\n");
+      }
+    }
   }
 
+  QFileInfo fi(fin->name);
+  qint64 size = fi.size();
+  if ((len + fit_data.len + 2) != size) {
+    Warning() << MYNAME ": File size" << size << "is not expected given header len" << len << ", data length" << fit_data.len << "and a 2 byte file CRC.";
+  } else if (global_opts.debug_level >= 1) {
+    debug_print(1, MYNAME ": File size matches expectations from information in the header.\n");
+  }
+
+  gbfseek(fin, len, SEEK_SET);
+
   fit_data.global_utc_offset = 0;
 }
 
 uint8_t
 GarminFitFormat::fit_getuint8()
 {
-  if (fit_data.len == 0) {
-    // fail gracefully for GARMIN Edge 800 with newest firmware, seems to write a wrong record length
-    // for the last record.
-    //fatal(MYNAME ": record truncated: fit_data.len=0\n");
-    if (global_opts.debug_level >= 1) {
-      warning("%s: record truncated: fit_data.len=0\n", MYNAME);
-    }
-    return 0;
+  if (fit_data.len < 1) {
+    throw ReaderException("record truncated: expecting char[1], but only got " + std::to_string(fit_data.len) + ".");
   }
   int val = gbfgetc(fin);
   if (val == EOF) {
-    fatal(MYNAME ": unexpected end of file with fit_data.len=%d\n",fit_data.len);
+    throw ReaderException("unexpected end of file with fit_data.len=" + std::to_string(fit_data.len) + ".");
   }
-  fit_data.len--;
-  return (uint8_t)val;
+  --fit_data.len;
+  return static_cast<uint8_t>(val);
 }
 
 uint16_t
@@ -161,10 +188,12 @@ GarminFitFormat::fit_getuint16()
   char buf[2];
 
   if (fit_data.len < 2) {
-    fatal(MYNAME ": record truncated: expecting char[2], but only got %d\n",fit_data.len);
+    throw ReaderException("record truncated: expecting char[2], but only got " + std::to_string(fit_data.len) + ".");
+  }
+  gbsize_t count = gbfread(buf, 2, 1, fin);
+  if (count != 1) {
+    throw ReaderException("unexpected end of file with fit_data.len=" + std::to_string(fit_data.len) + ".");
   }
-  is_fatal(gbfread(buf, 2, 1, fin) != 1,
-           MYNAME ": unexpected end of file with fit_data.len=%d\n",fit_data.len);
   fit_data.len -= 2;
   if (fit_data.endian) {
     return be_read16(buf);
@@ -179,10 +208,12 @@ GarminFitFormat::fit_getuint32()
   char buf[4];
 
   if (fit_data.len < 4) {
-    fatal(MYNAME ": record truncated: expecting char[4], but only got %d\n",fit_data.len);
+    throw ReaderException("record truncated: expecting char[4], but only got " + std::to_string(fit_data.len) + ".");
+  }
+  gbsize_t count = gbfread(buf, 4, 1, fin);
+  if (count != 1) {
+    throw ReaderException("unexpected end of file with fit_data.len=" + std::to_string(fit_data.len) + ".");
   }
-  is_fatal(gbfread(buf, 4, 1, fin) != 1,
-           MYNAME ": unexpected end of file with fit_data.len=%d\n",fit_data.len);
   fit_data.len -= 4;
   if (fit_data.endian) {
     return be_read32(buf);
@@ -195,23 +226,21 @@ void
 GarminFitFormat::fit_parse_definition_message(uint8_t header)
 {
   int local_id = header & 0x0f;
-  fit_message_def* def = &fit_data.message_def[local_id];
-
-  def->fields = QList<fit_field_t>();
+  fit_message_def def;
 
   // first byte is reserved.  It's usually 0 and we don't know what it is,
   // but we've seen some files that are 0x40.  So we just read it and toss it.
   (void) fit_getuint8();
 
   // second byte is endianness
-  def->endian = fit_getuint8();
-  if (def->endian > 1) {
-    warning(MYNAME ": Unusual endian field (interpreting as big endian): %d\n",def->endian);
+  def.endian = fit_getuint8();
+  if (def.endian > 1) {
+    throw ReaderException(QString("Bad endian field 0x%1 at file position 0x%2.").arg(def.endian, 0, 16).arg(gbftell(fin) - 1, 0, 16).toStdString());
   }
-  fit_data.endian = def->endian;
+  fit_data.endian = def.endian;
 
   // next two bytes are the global message number
-  def->global_id = fit_getuint16();
+  def.global_id = fit_getuint16();
 
   // byte 5 has the number of records in the remainder of the definition message
   int num_fields = fit_getuint8();
@@ -229,7 +258,7 @@ GarminFitFormat::fit_parse_definition_message(uint8_t header)
       debug_print(8,"%s: record %d  ID: %d  SIZE: %d  TYPE: %d  fit_data.len=%d\n",
                   MYNAME, i, field.id, field.size, field.type, fit_data.len);
     }
-    def->fields.append(field);
+    def.fields.append(field);
   }
 
   // If we have developer fields (since version 2.0) they must be read too
@@ -259,26 +288,26 @@ GarminFitFormat::fit_parse_definition_message(uint8_t header)
     if (global_opts.debug_level >= 8) {
       debug_print(8,"%s: definition message contains %d developer records\n",MYNAME, numOfDevFields);
     }
-    if (numOfDevFields == 0) {
-      return;
-    }
-
-    int numOfFields = num_fields + numOfDevFields;
-    for (int i = num_fields; i < numOfFields; ++i) {
-      int id   = fit_getuint8();
-      int size = fit_getuint8();
-      int type = fit_getuint8();
-      fit_field_t field = {id, size, type};
-      if (global_opts.debug_level >= 8) {
-        debug_print(8,"%s: developer record %d  ID: %d  SIZE: %d  TYPE: %d  fit_data.len=%d\n",
-                    MYNAME, i - num_fields, field.id, field.size, field.type, fit_data.len);
+    if (numOfDevFields > 0) {
+      int numOfFields = num_fields + numOfDevFields;
+      for (int i = num_fields; i < numOfFields; ++i) {
+        int id   = fit_getuint8();
+        int size = fit_getuint8();
+        int type = fit_getuint8();
+        fit_field_t field = {id, size, type};
+        if (global_opts.debug_level >= 8) {
+          debug_print(8,"%s: developer record %d  ID: %d  SIZE: %d  TYPE: %d  fit_data.len=%d\n",
+                      MYNAME, i - num_fields, field.id, field.size, field.type, fit_data.len);
+        }
+        // Because we parse developer fields like normal fields and we do not want
+        // that the field id interfere which valid id's from the normal fields
+        field.id = kFieldInvalid;
+        def.fields.append(field);
       }
-      // Because we parse developer fields like normal fields and we do not want
-      // that the field id interfere which valid id's from the normal fields
-      field.id = kFieldInvalid;
-      def->fields.append(field);
     }
   }
+
+  fit_data.message_def.insert(local_id, def);
 }
 
 uint32_t
@@ -645,14 +674,26 @@ void
 GarminFitFormat::fit_parse_data_message(uint8_t header)
 {
   int local_id = header & 0x0f;
-  fit_parse_data(fit_data.message_def[local_id], 0);
+  if (fit_data.message_def.contains(local_id)) {
+    fit_parse_data(fit_data.message_def.value(local_id), 0);
+  } else {
+    throw ReaderException(
+      QString("Message %1 hasn't been defined before being used at file position 0x%2.").
+      arg(local_id).arg(gbftell(fin) - 1, 0, 16).toStdString());
+  }
 }
 
 void
 GarminFitFormat::fit_parse_compressed_message(uint8_t header)
 {
   int local_id = (header >> 5) & 3;
-  fit_parse_data(fit_data.message_def[local_id], header & 0x1f);
+  if (fit_data.message_def.contains(local_id)) {
+    fit_parse_data(fit_data.message_def.value(local_id), header & 0x1f);
+  } else {
+    throw ReaderException(
+      QString("Compressed message %1 hasn't been defined before being used at file position 0x%2.").
+      arg(local_id).arg(gbftell(fin) - 1, 0, 16).toStdString());
+  }
 }
 
 /*******************************************************************************
@@ -661,6 +702,7 @@ GarminFitFormat::fit_parse_compressed_message(uint8_t header)
 void
 GarminFitFormat::fit_parse_record()
 {
+  gbsize_t position = gbftell(fin);
   uint8_t header = fit_getuint8();
   // high bit 7 set -> compressed message (0 for normal)
   // second bit 6 set -> 0 for data message, 1 for definition message
@@ -672,25 +714,53 @@ GarminFitFormat::fit_parse_record()
   // bits 3..0 -> local message type
   if (header & 0x80) {
     if (global_opts.debug_level >= 6) {
-      debug_print(6,"%s: got compressed message at fit_data.len=%d", MYNAME, fit_data.len);
+      debug_print(6,"%s: got compressed message at file position 0x%x, fit_data.len=%d", MYNAME, position, fit_data.len);
       debug_print(0," ...local message type 0x%X\n", header&0x0f);
     }
     fit_parse_compressed_message(header);
   } else if (header & 0x40) {
     if (global_opts.debug_level >= 6) {
-      debug_print(6,"%s: got definition message at fit_data.len=%d", MYNAME, fit_data.len);
+      debug_print(6,"%s: got definition message at file position 0x%x, fit_data.len=%d", MYNAME, position, fit_data.len);
       debug_print(0," ...local message type 0x%X\n", header&0x0f);
     }
     fit_parse_definition_message(header);
   } else {
     if (global_opts.debug_level >= 6) {
-      debug_print(6,"%s: got data message at fit_data.len=%d", MYNAME, fit_data.len);
+      debug_print(6,"%s: got data message at file position 0x%x, fit_data.len=%d", MYNAME, position, fit_data.len);
       debug_print(0," ...local message type 0x%X\n", header&0x0f);
     }
     fit_parse_data_message(header);
   }
 }
 
+void
+GarminFitFormat::fit_check_file_crc() const
+{
+  // Check file CRC
+
+  gbsize_t position = gbftell(fin);
+
+  uint16_t crc = 0;
+  gbfseek(fin, 0, SEEK_SET);
+  while (true) {
+    int data = gbfgetc(fin);
+    if (data == EOF) {
+      break;
+    }
+    crc = fit_crc16(data, crc);
+  }
+  if (crc != 0) {
+    Warning().nospace() << MYNAME ": File CRC mismatch in file " <<  fin->name << ".";
+    if (!opt_recoverymode) {
+      Fatal().nospace() << MYNAME ": File " << fin->name << " is corrupt.  Use recoverymode option at your risk.";
+    }
+  } else if (global_opts.debug_level >= 1) {
+    debug_print(1, MYNAME ": File CRC verified.\n");
+  }
+
+  gbfseek(fin, position, SEEK_SET);
+}
+
 /*******************************************************************************
 * fit_read- global entry point
 * - parse the header
@@ -699,6 +769,8 @@ GarminFitFormat::fit_parse_record()
 void
 GarminFitFormat::read()
 {
+  fit_check_file_crc();
+
   fit_parse_header();
 
   fit_data.track = new route_head;
@@ -706,8 +778,17 @@ GarminFitFormat::read()
   if (global_opts.debug_level >= 1) {
     debug_print(1,"%s: starting to read data with fit_data.len=%d\n", MYNAME, fit_data.len);
   }
-  while (fit_data.len) {
-    fit_parse_record();
+  try {
+    while (fit_data.len) {
+      fit_parse_record();
+    }
+  } catch (ReaderException& e) {
+    if (opt_recoverymode) {
+      warning(MYNAME ": %s\n",e.what());
+      warning(MYNAME ": Aborting read and continuning processing.\n");
+    } else {
+      fatal(MYNAME ": %s  Use recoverymode option at your risk.\n",e.what());
+    }
   }
 }
 
index e5f697c42f3a8e4d7b058007b7e86c9ebf10b2ce..4cbd8d8cb25c5c9d710b767162863cab025e21cb 100644 (file)
 
 #include <cstdint>              // for uint8_t, uint16_t, uint32_t
 #include <deque>                // for deque
+#include <stdexcept>            // for runtime_error
 #include <utility>              // for pair
 #include <vector>               // for vector
 
+#include <QtCore/QHash>         // for QHash
 #include <QtCore/QList>         // for QList
 #include <QtCore/QString>       // for QString
 #include <QtCore/QVector>       // for QVector
@@ -82,12 +84,12 @@ private:
   /* Types */
 
   struct fit_field_t {
-  /* MSVC 2015 generates C2664 errors without some help. */
+    /* MSVC 2015 generates C2664 errors without some help. */
 #if defined(_MSC_VER) && (_MSC_VER < 1910) /* MSVC 2015 or earlier */
     fit_field_t() = default;
     fit_field_t(int i, int s, int t) : id(i), size(s), type(t) {}
 #endif
-    int id{};
+    int id {};
     int size{};
     int type{};
   };
@@ -104,7 +106,7 @@ private:
     route_head* track{nullptr};
     uint32_t last_timestamp{};
     uint32_t global_utc_offset{};
-    fit_message_def message_def[16];
+    QHash<int, fit_message_def> message_def;
   };
 
   struct FitCourseRecordPoint {
@@ -126,6 +128,11 @@ private:
     unsigned int course_point_type;
   };
 
+  class ReaderException : public std::runtime_error
+  {
+    using std::runtime_error::runtime_error;
+  };
+
   /* Constants */
 
 // constants for global IDs
@@ -218,6 +225,7 @@ private:
 
   static constexpr int kWriteHeaderLen = 12;
   static constexpr int kWriteHeaderCrcLen = 14;
+  static constexpr int kReadHeaderCrcLen = 14;
 
   static constexpr double kSynthSpeed = 10.0 * 1000 / 3600; /* speed in m/s */
 
@@ -233,6 +241,7 @@ private:
   void fit_parse_data_message(uint8_t header);
   void fit_parse_compressed_message(uint8_t header);
   void fit_parse_record();
+  void fit_check_file_crc() const;
   void fit_write_message_def(uint8_t local_id, uint16_t global_id, const std::vector<fit_field_t>& fields) const;
   static uint16_t fit_crc16(uint8_t data, uint16_t crc);
   void fit_write_timestamp(const gpsbabel::DateTime& t) const;
@@ -255,6 +264,7 @@ private:
   /* Data Members */
 
   char* opt_allpoints = nullptr;
+  char* opt_recoverymode = nullptr;
   int lap_ct = 0;
   bool new_trkseg = false;
   bool write_header_msgs = false;
@@ -265,6 +275,11 @@ private:
       "Read all points even if latitude or longitude is missing",
       nullptr, ARGTYPE_BOOL, ARG_NOMINMAX, nullptr
     },
+    {
+      "recoverymode", &opt_recoverymode,
+      "Attempt to recovery data from corrupt file",
+      nullptr, ARGTYPE_BOOL, ARG_NOMINMAX, nullptr
+    },
   };
 
   const std::vector<std::pair<QString, int> > kCoursePointTypeMapping = {
index 79d05c79f93b258451ec44024904977fd4056573..c3316d0f37e0a30a710d0f2cc6fb70e9b54f45e2 100644 (file)
@@ -274,6 +274,8 @@ file        -wrw--  garmin_fit      fit     Flexible and Interoperable Data Transfer (FIT) Activi
        https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin_fit.html
 option garmin_fit      allpoints       Read all points even if latitude or longitude is missing        boolean                         https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin_fit.html#fmt_garmin_fit_o_allpoints
 
+option garmin_fit      recoverymode    Attempt to recovery data from corrupt file      boolean                         https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin_fit.html#fmt_garmin_fit_o_recoverymode
+
 file   rw----  flysight        csv     FlySight GPS File       xcsv
        https://www.gpsbabel.org/WEB_DOC_DIR/fmt_flysight.html
 option flysight        snlen   Max synthesized shortname length        integer         1               https://www.gpsbabel.org/WEB_DOC_DIR/fmt_flysight.html#fmt_flysight_o_snlen
index 09ca5dbbf29206b45021c53ad7cd5fb5415c3ae0..91962ad3a85b1153c0d9da2855b3c16977e9807c 100644 (file)
@@ -145,6 +145,7 @@ File Types (-i and -o options):
          timeadj               (integer sec or 'auto') Barograph to GPS time diff 
        garmin_fit            Flexible and Interoperable Data Transfer (FIT) Act
          allpoints             (0/1) Read all points even if latitude or longitude is m 
+         recoverymode          (0/1) Attempt to recovery data from corrupt file 
        flysight              FlySight GPS File
          snlen                 Max synthesized shortname length 
          snwhite               (0/1) Allow whitespace synth. shortnames 
index 3024edd4a7bd93648b1b36ea2866c04e775fab58..876317ca1468f348c77958e0505a352b48fb2223 100644 (file)
@@ -26,7 +26,7 @@ compare ${REFERENCE}/track/wahoo-element-bolt.gpx ${TMPDIR}/fit-sample-wahoo-ele
 gpsbabel -i garmin_fit -f ${REFERENCE}/track/garmin-oregon-700.fit -o gpx -F ${TMPDIR}/fit-sample-garmin-oregon-700.gpx
 compare ${REFERENCE}/track/garmin-oregon-700-output.gpx ${TMPDIR}/fit-sample-garmin-oregon-700.gpx
 
-gpsbabel -i garmin_fit -f ${REFERENCE}/track/lezyne_super_gps-garmin_fit-sample-bad-endian.fit -o gpx -F ${TMPDIR}/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx
+gpsbabel -i garmin_fit,recoverymode -f ${REFERENCE}/track/lezyne_super_gps-garmin_fit-sample-bad-endian.fit -o gpx -F ${TMPDIR}/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx 2>/dev/null
 compare ${REFERENCE}/track/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx ${TMPDIR}/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx
 
 #
diff --git a/xmldoc/formats/options/garmin_fit-recoverymode.xml b/xmldoc/formats/options/garmin_fit-recoverymode.xml
new file mode 100644 (file)
index 0000000..18c96cc
--- /dev/null
@@ -0,0 +1,13 @@
+<para>
+In the default mode the reader will issue a fatal error if it encounters indications of a corrupt file. These indications include:
+<itemizedlist>
+<listitem><para>a bad Header or File CRC</para></listitem>
+<listitem><para>a bad endian field</para></listitem>
+<listitem><para>an attempt to use a message type that hasn't been previously defined</para></listitem>
+<listitem><para>an attempt to read when the data section doesn't have sufficient data</para></listitem>
+<listitem><para>an attempt to read past the end of file</para></listitem>
+</itemizedlist>
+In recovery mode if we encounter a CRC error we will ignore it.  If we encounter one of the other errors we will abort
+read processing and continue. This allows any writer to use data that
+was recovered previous to the read abort.
+</para>